fix: handle in/notin operators in token search filters#1656
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 20 minutes and 37 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR extends OrgTokensRepository.addFilter to support "in" and "notin" operators by parsing comma-separated values (using strings) and generating TEXT-cast SQL IN/NOT IN predicates from the resulting list. ChangesFilter Operator Extension
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e693f884-d065-4e95-abdd-bb22f50d3118
📒 Files selected for processing (1)
internal/store/postgres/org_tokens_repository.go
Coverage Report for CI Build 26899856118Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.02%) to 43.138%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/store/postgres/org_tokens_repository.go (1)
171-176:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty value list yields
NOT IN ()/IN ()and silently matches every / no row.When the input is
""or just",",valuesends up empty and the predicate degrades into an always-true (notin) or always-false (in) clause. Consider bailing out with apostgres.ErrBadInput-wrapped error so the handler can map it to a 400 instead of returning unexpected rows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aede523b-db47-4675-94a0-ef0eb7b84c82
📒 Files selected for processing (1)
internal/store/postgres/org_tokens_repository.go
63412b4 to
dbc2c9c
Compare
Summary
in/notinoperator handling inOrgTokensRepository.addFilterinoperator requires splitting comma-separated string values into a slice for goqu'sIn()methodgoqu.Op{"in": "system.buy"}generates invalid SQL — it passes a raw string instead of a list toIN ()OrgBillingRepository.addRQLFiltersInQuerywhich already handles this correctlyRoot cause
The DataTable multiselect filter sends
operator: "in"withstringValue: "system.buy"(comma-separated for multiple values). TheaddFilterdefault case passes this directly to goqu asgoqu.Op{"in": filter.Value}, but goqu'sinneeds a slice, not a string. The fix splits the value by commas before passing toIn()/NotIn().Test plan
notinworks